Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: mark native image checks as required #1930

Merged
merged 1 commit into from
Dec 13, 2022
Merged

chore: mark native image checks as required #1930

merged 1 commit into from
Dec 13, 2022

Conversation

mpeddada1
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@mpeddada1 mpeddada1 requested review from a team as code owners June 29, 2022 14:36
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: spanner Issues related to the googleapis/java-spanner API. labels Jun 29, 2022
@ansh0l
Copy link
Contributor

ansh0l commented Jul 4, 2022

Hey @mpeddada1 , would you have any stats on how this new presubmit is doing? As in, how often was it failing when any of the merges got done previously? If not, I would want to monitor for an additional month or so before making it mandatory.

@ansh0l ansh0l added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 4, 2022
@mpeddada1
Copy link
Contributor Author

Hey @ansh0l, just looked into the recently merged PRs in this repository. I haven't observed failures specific to native image compilation so far but I have noticed the ITInstanceAdminTest#updateInstance and ITInstanceAdminTest#updateInstanceViaEntity tests in these presubmits being pretty flaky. I'm wondering if this could be related to #1626?

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 8, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 8, 2022
@ansh0l
Copy link
Contributor

ansh0l commented Jul 12, 2022

cc @rajatbhatta to take a look at the related issue

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 12, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 12, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 13, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@mpeddada1
Copy link
Contributor Author

Thanks @ansh0l! are we considering #1626 as a prerequisite to marking these checks as required?

This was referenced Jul 19, 2022
Merged
Merged
@mpeddada1
Copy link
Contributor Author

Hi @ansh0l, checking back on this PR. Now that #1930 is fixed, are we okay to remove the do-merge-label?

@ansh0l
Copy link
Contributor

ansh0l commented Aug 29, 2022

Thanks @mpeddada1 ! We can monitor for another couple of weeks, and then merge this.

@mpeddada1
Copy link
Contributor Author

mpeddada1 commented Dec 13, 2022 •
edited
Loading

Hey @ansh0l @rajatbhatta looks like we haven't had a chance to merge this PR in yet. Could we get your review on this? It'll help us gate releases (without manual verification) on the compatibility with native image compilation.

@rajatbhatta rajatbhatta removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 13, 2022
@rajatbhatta rajatbhatta merged commit 3a8ceb4 into main Dec 13, 2022
@rajatbhatta rajatbhatta deleted the require-graal-check branch December 13, 2022 16:06
@rajatbhatta
Copy link
Contributor

Hi @mpeddada1: Merged this PR. Thank you!

@mpeddada1
Copy link
Contributor Author

Thank you @rajatbhatta!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajatbhatta rajatbhatta rajatbhatta approved these changes

@ansh0l ansh0l Awaiting requested review from ansh0l

Assignees
No one assigned
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: xs Pull request size is extra small.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mpeddada1 @ansh0l @rajatbhatta @yoshi-kokoro